src/SecurityError.js (2)

13-28: Consider extracting the logger selection logic to reduce duplication.

Both createSanitizedError and createSanitizedHttpError contain identical logic for selecting the logger. This violates the DRY principle and makes maintenance harder.

Apply this diff to extract the common logic:

+/**
+ * Selects the appropriate logger from the provided parameter
+ * @param {Object} loggerOrConfig - Optional logger instance or config object
+ * @returns {Object} The selected logger instance
+ */
+function selectLogger(loggerOrConfig) {
+  if (loggerOrConfig) {
+    if (loggerOrConfig.loggerController) {
+      return loggerOrConfig.loggerController;
+    } else if (loggerOrConfig.error) {
+      return loggerOrConfig;
+    }
+  }
+  return defaultLogger;
+}
+
 export function createSanitizedError(errorCode, detailedMessage, loggerOrConfig = null) {
-  let log = defaultLogger;
-  if (loggerOrConfig) {
-    if (loggerOrConfig.loggerController) {
-      log = loggerOrConfig.loggerController;
-    } else if (loggerOrConfig.error) {
-      // It's a logger instance
-      log = loggerOrConfig;
-    }
-  }
+  const log = selectLogger(loggerOrConfig);
 
   // Keep log on server side
   log.error('Security error:', detailedMessage);
 
   return new Parse.Error(errorCode, 'Permission denied');
 }

 export function createSanitizedHttpError(statusCode, detailedMessage, loggerOrConfig = null) {
-  let log = defaultLogger;
-  if (loggerOrConfig) {
-    if (loggerOrConfig.loggerController) {
-      log = loggerOrConfig.loggerController;
-    } else if (loggerOrConfig.error) {
-      log = loggerOrConfig;
-    }
-  }
+  const log = selectLogger(loggerOrConfig);
 
   // Keep log on server side
   log.error('Security error:', detailedMessage);
 
   const error = new Error();
   error.status = statusCode;
   error.message = 'Permission denied';
   return error;
 }

Also applies to: 39-56

---

1-57: Suggest improved PR title following Angular commit convention.

Based on learnings

The current PR title "feat: sanitize error for security" could be more descriptive and follow the Angular commit convention more precisely. Consider:

feat(security): sanitize error messages to prevent information disclosure

This title:

• Includes the scope (security)
• Clearly conveys the security improvement
• Makes a meaningful changelog entry for developers

src/Routers/FilesRouter.js (1)

8-9: Avoid double‑logging the security error in getHandler

createSanitizedError already logs 'Security error:' with the detailed message, so the explicit log.error('Security error:', detailedError); causes duplicate log entries for the same event.

You can rely on createSanitizedError’s internal logging and drop both the explicit log call and the extra logger argument:

-import { createSanitizedError } from '../SecurityError';
-import defaultLogger from '../logger';
+import { createSanitizedError } from '../SecurityError';
@@
     if (!config) {
       res.status(403);
-      const detailedError = 'Invalid application ID.';
-      const log = defaultLogger;
-      log.error('Security error:', detailedError);
-      const err = createSanitizedError(Parse.Error.OPERATION_FORBIDDEN, detailedError, log);
+      const detailedError = 'Invalid application ID.';
+      const err = createSanitizedError(Parse.Error.OPERATION_FORBIDDEN, detailedError);
       res.json({ code: err.code, error: err.message });
       return;
     }

This keeps behavior the same (code + sanitized message) while logging only once.

Also applies to: 48-52

spec/ParseGraphQLServer.spec.js (1)

47-61: PR title: suggest Angular-style scoped variant

To make the changelog entry more informative and follow the Angular convention from this repo, consider updating the PR title to something like:

feat(security): sanitize permission error messages

This reflects the security impact and the main behavior change more clearly.

src/RestQuery.js (1)

432-435: Use config-aware logger consistently when sanitizing errors in RestQuery

denyProtectedFields correctly derives log from this.config.loggerController || defaultLogger, but validateClientClassCreation currently calls createSanitizedError without passing any logger/config, so it always falls back to the global default.

To keep logging consistent and app-scoped, consider:

-        if (hasClass !== true) {
-          throw createSanitizedError(
-            Parse.Error.OPERATION_FORBIDDEN,
-            'This user is not allowed to access ' + 'non-existent class: ' + this.className
-          );
-        }
+        if (hasClass !== true) {
+          const detailedError =
+            'This user is not allowed to access non-existent class: ' + this.className;
+          const log = (this.config && this.config.loggerController) || defaultLogger;
+          throw createSanitizedError(
+            Parse.Error.OPERATION_FORBIDDEN,
+            detailedError,
+            log
+          );
+        }

Also applies to: 811-816

src/triggers.js (1)

4-6: Validator now distinguishes security vs generic validation errors

The updated maybeRunValidator logic:

• Correctly treats "Master key is required" and "Please login to continue" failures as security-sensitive, logging the detailed reason via defaultLogger and returning a generic VALIDATION_ERROR with message 'Permission denied'.
• Preserves the existing 'Validation failed.' behavior for non-security validation problems.

If you want to further centralize sanitization, you could consider reusing the shared helper instead of hand-rolling:

-        if (isSecurityError) {
-          // Log detailed error server-side and sanitize for client
-          defaultLogger.error('Security validation error:', errorMessage);
-          error = resolveError(e, {
-            code: Parse.Error.VALIDATION_ERROR,
-            message: 'Permission denied',
-          });
-        } else {
+        if (isSecurityError) {
+          defaultLogger.error('Security validation error:', errorMessage);
+          error = resolveError(e, {
+            code: Parse.Error.VALIDATION_ERROR,
+            message: 'Permission denied',
+          });
+        } else {

(or swap in createSanitizedError if you decide to standardize on that helper for all security errors.)

Also applies to: 696-735

src/RestWrite.js (1)

20-22: RestWrite sanitization is sound; consider minor logger and read-only consistency tweaks

The new sanitized error paths in RestWrite are functionally correct:

• validateClientClassCreation and the unauthenticated _User update guard now log detailed reasons via (this.config && this.config.loggerController) || defaultLogger and expose only a generic permission error to clients.
• checkRestrictedFields correctly blocks client-side updates to emailVerified with OPERATION_FORBIDDEN.

A few small follow-ups would make this file more consistent with the rest of the PR:

1. Use the config-aware logger in checkRestrictedFields

Right now it relies on createSanitizedError’s internal default logger. To align with the other call sites:

   if (!this.auth.isMaintenance && !this.auth.isMaster && 'emailVerified' in this.data) {
-    throw createSanitizedError(Parse.Error.OPERATION_FORBIDDEN, 'Clients aren\'t allowed to manually update email verification.');
+    const log = (this.config && this.config.loggerController) || defaultLogger;
+    throw createSanitizedError(
+      Parse.Error.OPERATION_FORBIDDEN,
+      "Clients aren't allowed to manually update email verification.",
+      log
+    );
   }

2. Deduplicate logger imports

You currently import the same default export twice:

import logger from './logger';
import defaultLogger from './logger';

You can keep just one default import and reuse it (e.g. stick with logger and drop defaultLogger, or vice versa), which avoids confusion and potential lint warnings.

3. Optional: sanitize the read-only masterKey constructor guard

The constructor still throws a raw Parse.Error mentioning readOnlyMasterKey. Given that SharedRest.enforceRoleSecurity should block external read-only writes first, this is mostly an internal edge case, but you may still want to route it through createSanitizedError for consistency with other read-only enforcement paths.

Also applies to: 191-216, 662-669, 1456-1463

src/SharedRest.js (3)

1-15: Configurable logger integration and signature change look correct; consider minor DRY refactor

• Importing createSanitizedError / defaultLogger and extending enforceRoleSecurity to accept config = null is backward compatible and aligns with the new sanitization helper.
• To avoid repeating config || defaultLogger in each branch, consider computing it once at the top of the function, e.g. const loggerOrConfig = config || defaultLogger;, and reuse it below.

---

17-20: Sanitized errors across all forbidden paths align with the security objective; optional helper could reduce duplication

• Each forbidden-operation path now builds a clear detailedError, selects loggerOrConfig, and throws createSanitizedError(Parse.Error.OPERATION_FORBIDDEN, …, loggerOrConfig), which should log server-side and return a generic “Permission denied” to clients—this matches the goal of not leaking details while preserving the error code.
• The three branches share the same pattern; you could optionally factor a tiny local helper (or reuse a precomputed loggerOrConfig) to reduce duplication and keep future changes to the sanitization behavior in one place.

Also applies to: 29-31, 36-38

---

1-44: PR-level: title suggestion and documentation note for this security behavior change

• Given this is a security-focused behavioral change that developers should notice in the changelog, consider renaming the PR to an Angular-style title such as:
  feat(security): sanitize permission errors
• It may also be worth adding a short note to the security/docs/README explaining that certain authorization errors are now always surfaced as generic “Permission denied” while details are only logged server-side, so integrators know not to rely on error message text.

Based on learnings

src/Routers/SchemasRouter.js (1)

8-9: Sanitized error utilities imported correctly; minor DRY opportunity and PR title suggestion

Imports for createSanitizedError and defaultLogger look good and match the shared helper pattern. To avoid repeating the same logger expression in multiple handlers, you could optionally introduce a small helper (e.g. const log = req.config?.loggerController || defaultLogger;) or pass req.config directly into createSanitizedError and let it resolve the logger internally, for slightly cleaner code.

Side note for the PR itself: an Angular-style title that would read well in the changelog could be:

fix(security): sanitize read-only schema error messages

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

src/SecurityError.js (3)

12-21: Make logger resolution more defensive and DRY.

Both helpers duplicate the same logger resolution logic and treat any truthy loggerOrConfig.error as a logger, even if it’s not a function. That’s fine for the expected config / logger inputs, but it’s brittle for accidental misuse.

Consider extracting a small resolveLogger(loggerOrConfig) helper that:

• Returns loggerOrConfig.loggerController when present.
• Otherwise returns loggerOrConfig only if typeof loggerOrConfig.error === 'function'.
• Falls back to defaultLogger in all other cases.

That removes duplication and avoids ever ending up with an object that doesn’t actually implement error().

Also applies to: 38-46

---

23-27: Optional: enrich server‑side logs with error code / status as metadata.

Right now the log entry only includes the string message; the returned error carries the code/status, but that information is not in the log call itself. For security auditing and debugging it can be useful to log the numeric errorCode / statusCode as well (e.g. as an additional argument or metadata object), while still keeping the client message sanitized.

This is not required for correctness, but worth considering for more informative security logs across the codebase.

Also applies to: 48-54

---

1-55: PR‑level: align title with Angular convention and consider documenting the new error behavior.

Per the repo’s conventions, you may want to adjust the PR title to an Angular‑style variant that’s clearer in the changelog, for example:

• feat(security): sanitize permission error responses

Also, since this changes client‑visible error messages (even though codes stay the same), consider a short note in the README or upgrade notes explaining that certain security/permission errors now always return "Permission denied" while detailed reasons are only available in server logs.

src/SharedRest.js (1)

9-10: Access checks look correct; consider centralizing logger resolution.

The refactor keeps the existing access rules intact while routing all denials through createSanitizedError, which is exactly what we want for hiding detailed messages from clients and logging them server‑side. The new config parameter is optional, so existing callers remain valid and fall back to defaultLogger.

You could optionally compute const loggerOrConfig = config || defaultLogger; once near the top of enforceRoleSecurity and reuse it in all three branches to avoid repetition, but that’s purely cosmetic and not required.

Also applies to: 13-19, 28-31, 35-37

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

src/rest.js (1)

171-173: Sanitize the early _User delete guard to align with the rest of the file.

Line 172 still throws a raw Parse.Error(Parse.Error.SESSION_MISSING, 'Insufficient auth to delete user'), exposing operation detail. This contradicts the sanitization applied to handleSessionMissingError at lines 329-332.

Apply this diff to sanitize the error:

   if (className === '_User' && auth.isUnauthenticated()) {
-    throw new Parse.Error(Parse.Error.SESSION_MISSING, 'Insufficient auth to delete user');
+    const { createSanitizedError } = require('./SecurityError');
+    const detailedError = 'Insufficient auth to delete user';
+    throw createSanitizedError(Parse.Error.SESSION_MISSING, detailedError);
   }

Based on learnings

src/GraphQL/parseGraphQLUtils.js (2)

5-12: Master-key enforcement now goes through createSanitizedError; verify client message is actually sanitized.

Routing this OPERATION_FORBIDDEN through createSanitizedError is the right direction for centralizing and sanitizing permission errors, while preserving the error code. One thing to double‑check against #8958: if createSanitizedError forwards its message argument to the client unchanged, this would still expose "unauthorized: master key is required". In that case, either:

• have createSanitizedError sanitize to a generic message (e.g. "unauthorized"), or
• pass a pre‑sanitized message here and log the detailed context inside createSanitizedError.

It would be good to have a GraphQL test asserting that the client no longer sees "master key is required" in this path.

---

1-57: PR meta: Angular-style title suggestion and docs note.

For the changelog, an Angular-style title that reflects this behavior change could be:

• feat(security): sanitize permission errors for GraphQL

Also, since this subtly changes what clients see on authorization failures, consider a brief note in the README or security/operations docs (e.g., “detailed permission error messages are now logged server-side and hidden from clients”), even if no new option is added.

src/GraphQL/loaders/schemaMutations.js (1)

34-41: Read-only masterKey error is now sanitized in CreateClass.

The implementation correctly uses createSanitizedError to sanitize the error message. However, unlike similar changes in other files (e.g., src/RestWrite.js lines 204-210), this doesn't pass a config-aware logger. The error will still be sanitized, but detailed logging may use a default logger instead of the config's logger.

If you want consistent logging with the config's logger, consider:

         const { config, auth } = context;
 
         enforceMasterKeyAccess(auth, config);
 
         if (auth.isReadOnly) {
+          const log = (config && config.loggerController) || defaultLogger;
           throw createSanitizedError(
             Parse.Error.OPERATION_FORBIDDEN,
             "read-only masterKey isn't allowed to create a schema.",
+            log
           );
         }

(Same pattern applies to UpdateClass at lines 86-89 and DeleteClass at lines 137-140.)

src/Controllers/SchemaController.js (1)

1408-1413: Consider simplifying the error construction pattern.

Per past review feedback, these could be more compact without the intermediate detailedError variable:

       if (!aclGroup || aclGroup.length == 0) {
-        const detailedError = 'Permission denied, user needs to be authenticated.';
-        throw createSanitizedError(Parse.Error.OBJECT_NOT_FOUND, detailedError);
+        throw createSanitizedError(
+          Parse.Error.OBJECT_NOT_FOUND,
+          'Permission denied, user needs to be authenticated.'
+        );
       } else if (aclGroup.indexOf('*') > -1 && aclGroup.length == 1) {
-        const detailedError = 'Permission denied, user needs to be authenticated.';
-        throw createSanitizedError(Parse.Error.OBJECT_NOT_FOUND, detailedError);
+        throw createSanitizedError(
+          Parse.Error.OBJECT_NOT_FOUND,
+          'Permission denied, user needs to be authenticated.'
+        );
       }

This same pattern applies to lines 1426-1427 and 1447-1448 as well.

Based on learnings

src/RestWrite.js (2)

667-669: Good: compact error construction pattern.

The single-line createSanitizedError call is concise and readable, though it doesn't pass a config-aware logger like the implementation at lines 204-210.

For consistency with line 205, consider passing a config-aware logger:

-  if (!this.auth.isMaintenance && !this.auth.isMaster && 'emailVerified' in this.data) {
-    throw createSanitizedError(Parse.Error.OPERATION_FORBIDDEN, 'Clients aren\'t allowed to manually update email verification.');
+  if (!this.auth.isMaintenance && !this.auth.isMaster && 'emailVerified' in this.data) {
+    const log = (this.config && this.config.loggerController) || defaultLogger;
+    throw createSanitizedError(
+      Parse.Error.OPERATION_FORBIDDEN,
+      'Clients aren\'t allowed to manually update email verification.',
+      log
+    );
   }

---

1456-1462: User modification error is sanitized, but logger parameter could be added.

The implementation correctly uses createSanitizedError, but unlike the pattern at lines 204-210, it doesn't pass a config-aware logger.

For consistency, consider:

     if (this.className === '_User' && this.query && this.auth.isUnauthenticated()) {
       const detailedError = `Cannot modify user ${this.query.objectId}.`;
+      const log = (this.config && this.config.loggerController) || defaultLogger;
       throw createSanitizedError(
         Parse.Error.SESSION_MISSING,
         detailedError,
+        log
       );
     }

src/Routers/PushRouter.js (2)

13-16: Sanitized error use looks correct; optionally align logging and tests

Using createSanitizedError with Parse.Error.OPERATION_FORBIDDEN here keeps the error code stable while delegating message sanitization, which matches the objective of hiding detailed authorization reasons from clients.

Two optional follow-ups:

• If createSanitizedError supports receiving a logger or request context, consider passing in req.config.loggerController (or a derived logger) so read-only masterKey misuse on /push is logged consistently with other sanitized errors in this PR.
• Ensure there is a test that exercises this read-only masterKey path and asserts the generic client-facing message while still preserving the correct error code.

---

1-85: PR title & documentation suggestion

To make the changelog clearer and follow Angular-style conventions, consider renaming the PR to something like:

feat(security): sanitize unauthorized error messages

Also consider a brief note in the changelog (or README, if you think it affects integrators) that certain security-related errors now return generic client messages while logging detailed diagnostics server-side, so users understand why previously detailed messages have become more generic.

Based on learnings

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

src/RestWrite.js (1)

1458-1462: Verify logger usage consistency (same concern as line 666).

Similar to the comment at line 666, this createSanitizedError call doesn't pass a logger parameter, while the call at line 205 does. Ensure this is intentional and that the detailed error message (including this.query.objectId) is properly logged server-side.

src/rest.js (1)

171-176: Unauthenticated _User delete still throws raw error, bypassing sanitization helper

The early guard:

if (className === '_User' && auth.isUnauthenticated()) {
  throw new Parse.Error(Parse.Error.SESSION_MISSING, 'Insufficient auth to delete user');
}

still sends a detailed message directly to the client instead of going through createSanitizedError, which is inconsistent with the new sanitized session/auth handling and the behavior in handleSessionMissingError.

To fully align delete‑user with the sanitization strategy, route this branch through createSanitizedError as well:

   if (className === '_User' && auth.isUnauthenticated()) {
-    throw new Parse.Error(Parse.Error.SESSION_MISSING, 'Insufficient auth to delete user');
+    const { createSanitizedError } = require('./Error');
+    throw createSanitizedError(
+      Parse.Error.SESSION_MISSING,
+      'Insufficient auth to delete user'
+    );
   }

(Alternatively, you could delegate to handleSessionMissingError if you prefer a single central helper.)

This avoids leaking operation details to clients while keeping richer information in the server logs.

src/TestUtils.js (1)

85-104: Make logger spy resilient to repeated usage and tightly-coupled prefix

The helper works for current tests, but it assumes spyOn(logger, 'error') is only ever called once per spec and that tests always run with process.env.TESTING set so the first log arg is exactly 'Sanitized error:'. If a spec calls getSanitizedErrorCall() twice on the same logger, Jasmine may throw for double-spying unless allowRespy(true) is configured, and any future change to the prefix would silently break all these tests.

To make this more robust, consider:

• Reusing an existing spy if one already exists instead of always calling spyOn.
• Centralizing the 'Sanitized error:' prefix (e.g. exported constant from src/Error.js) so tests and implementation cannot drift.

spec/AudienceRouter.spec.js (1)

5-5: AudienceRouter master-key tests comprehensively validate sanitized errors

All push_audiences authorization tests now assert a generic 'Permission denied' on the client and use getSanitizedErrorCall to confirm the detailed master-key message was logged, covering POST/GET/PUT/DELETE consistently. This is a solid, end‑to‑end check of the new sanitization behavior.

Also applies to: 266-339

spec/ParseFile.spec.js (1)

766-776: Tidy TestUtils import and confirm unauthorized file-path behavior

Two small points here:

1. Inside "does not crash on file request with invalid app ID", you re-require('../lib/TestUtils') and destructure getSanitizedErrorCall, even though it is already imported at the top of the file. You can use the existing import and drop the inline require to avoid redundancy.

2. For invalid file paths, the test still expects { error: 'unauthorized' } instead of the generic 'Permission denied' used for invalid application IDs. If the goal is that all unauthorized file routes share the same sanitized client message, it may be worth aligning this case as well; otherwise, clarifying in a comment or issue which cases are deliberately left as 'unauthorized' could prevent confusion later.

Also applies to: 782-788

src/Error.js (1)

1-44: Centralized sanitization helpers look good; consider minor hardening and docs/changelog

The new helpers encapsulate the pattern of:

• Logging the detailed message via defaultLogger.error(...), with a 'Sanitized error:' prefix under TESTING so specs can detect it.
• Returning a Parse.Error or plain Error that always exposes the generic 'Permission denied' string while preserving the original error or HTTP status code.

A few small follow‑ups to consider:

• Avoid duplicate logging logic: Extract a small internal helper (e.g. logSanitizedError(detailedMessage)) shared by both functions to keep the logging behavior in one place and reduce drift risk if the format ever changes.
• Be explicit about Parse dependency: If feasible within this codebase’s conventions, importing Parse (rather than relying on a global) would make createSanitizedError more self‑contained and tooling‑friendly.
• Lock in the test contract: The tests rely on the 'Sanitized error:' prefix only being used in TESTING. If any other code path ever starts logging with that same prefix, the TestUtils helper may get false positives; conversely, changing the prefix will break many specs. It may be worth documenting this contract in a short comment near the export or in CONTRIBUTING.

Given this is a security‑relevant behavior change visible to clients, please also:

• Add or update a short note in the appropriate docs/README or migration/changelog area to mention that certain authorization failures now return a generic "Permission denied" message while keeping error codes unchanged.

• Update the PR title to an Angular-style form suitable for the changelog, for example:

  feat(security): sanitize unauthorized error messages

src/RestWrite.js (1)

204-209: Consider using optional chaining for consistency.

The logger retrieval works correctly, but for consistency with the patterns in src/middlewares.js (lines 236, 246, 463, 536), consider using optional chaining:

Apply this diff:

-          const log = (this.config && this.config.loggerController) || defaultLogger;
+          const log = this.config?.loggerController || defaultLogger;

spec/schemas.spec.js (2)

7-8: Avoid double requiring ../lib/TestUtils

You now import ../lib/TestUtils twice (as TestUtils and via destructuring). To keep things consistent with other specs (e.g. spec/ParseAPI.spec.js) and avoid redundancy, consider a single destructured import:

-const TestUtils = require('../lib/TestUtils');
-const { getSanitizedErrorCall } = require('../lib/TestUtils');
+const { destroyAllDataPermanently, getSanitizedErrorCall } = require('../lib/TestUtils');

and then call destroyAllDataPermanently directly where needed.

---

170-190: Strengthen schema auth tests by asserting error code as well

These two tests now correctly assert the sanitized message 'Permission denied', but they still rely only on status + message. To better pin down the behavior of the sanitized error, also assert the error code (e.g. Parse.Error.OPERATION_FORBIDDEN) in both tests.

   }).then(fail, response => {
     expect(response.status).toEqual(403);
-    expect(response.data.error).toEqual('Permission denied');
+    expect(response.data.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
+    expect(response.data.error).toEqual('Permission denied');

Apply the same pattern to the following test that covers /schemas with REST key.

spec/Schema.spec.js (1)

1449-1727: Consider asserting error codes in required-auth tests as well

The required-auth tests (find, create, get, find not authenticated) now:

• Assert the generic 'Permission denied' message.
• Use getSanitizedErrorCall to verify the detailed log message 'Permission denied, user needs to be authenticated.'.

To further harden these tests against regressions, you may want to also assert the error code (e.g., Parse.Error.OPERATION_FORBIDDEN) alongside the message in each of these cases, similar to the CLP tests above.

-        e => {
-          expect(e.message).toEqual('Permission denied');
+        e => {
+          expect(e.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
+          expect(e.message).toEqual('Permission denied');
           sanitizedErrorCall.checkMessage(
             'Permission denied, user needs to be authenticated.',
             callCountBefore
           );

This is optional but would make the required-auth suite consistent with the other permission tests.

spec/ParseAPI.spec.js (1)

1713-1733: Purge-without-master-key test correctly exercises sanitized error; consider tightening assertions

The updated test now:

• Uses getSanitizedErrorCall and callCountBefore.
• Asserts the client-facing response.data.error is 'Permission denied'.
• Verifies the logged detailed message contains 'unauthorized: master key is required'.

To fully specify the behavior, you could also assert the HTTP status and/or error code, e.g.:

   .catch(response => {
-    expect(response.data.error).toEqual('Permission denied');
+    expect(response.status).toBe(403);
+    expect(response.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
+    expect(response.data.error).toEqual('Permission denied');
     sanitizedErrorCall.checkMessage(
       'unauthorized: master key is required',
       callCountBefore
     );

Not required, but this would make the test more robust.

src/Routers/GraphQLRouter.js (1)

4-24: Read‑only master key now returns a sanitized forbidden error for GraphQL config updates

Using:

throw createSanitizedError(
  Parse.Error.OPERATION_FORBIDDEN,
  "read-only masterKey isn't allowed to update the GraphQL config.",
);

on req.auth.isReadOnly ensures:

• Clients only see a generic “Permission denied” (via the helper).
• Logs still carry the detailed reason, matching the rest of the sanitization work.

For the changelog, an Angular-style PR title that reflects this broader change could be:

feat(security): sanitize permission errors

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9858
File: src/GraphQL/ParseGraphQLServer.js:176-178
Timestamp: 2025-09-21T15:43:32.265Z
Learning: The GraphQL playground feature in ParseGraphQLServer.js (applyPlayground method) is intended for development environments only, which is why it includes the master key in client-side headers.

Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.

src/RestWrite.js (1)

1-1861: Consider improving the PR title per Angular commit convention.

Based on learnings, the PR title should follow the Angular commit convention format (type(scope): description) and make a meaningful changelog entry for developers.

Current title: feat: Sanitize error for security

Suggested improvement: fix(security): sanitize authorization error messages to prevent information disclosure

This better conveys that:

• It's a security fix (not a new feature)
• It addresses information disclosure
• It's specifically about authorization errors
• It provides value as a changelog entry

Based on learnings

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

src/Routers/ClassesRouter.js (1)

7-7: Remove unused import.

The defaultLogger import is not used anywhere in this file. If createSanitizedError handles logging internally (as suggested by the PR objectives), this import can be removed.

Apply this diff to remove the unused import:

-import defaultLogger from '../logger';

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.

Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.